Skip to content

[O2B-1534] Migrate log overview to use filtering model pattern#2083

Open
NarrowsProjects wants to merge 64 commits intomainfrom
improv/O2B-1534/Migrate-Log-Overview-to-use-FilteringModel-pattern
Open

[O2B-1534] Migrate log overview to use filtering model pattern#2083
NarrowsProjects wants to merge 64 commits intomainfrom
improv/O2B-1534/Migrate-Log-Overview-to-use-FilteringModel-pattern

Conversation

@NarrowsProjects
Copy link
Copy Markdown
Collaborator

@NarrowsProjects NarrowsProjects commented Feb 20, 2026

I have a JIRA ticket

  • branch and/or PR name(s) include(s) JIRA ID
  • issue has "Fix version" assigned
  • issue "Status" is set to "In review"
  • PR labels are selected

Notable changes for users:

  • Individual from-to selectors on the logs page have been replaced with a single field that will open the standard time range selector
  • All open-input forms apply their filters when focus is lost on them.

Notable changes for developers:

  • All filters have been moved to a filteringModel in logsOverviewModel
  • The filters for runs, lhcfills and environments have been renamed under the hood to match getAllRunsUseCase:
    • runs => runNumbers
    • lhcFills => fillNumbers
    • environments => environmentIds

Changes made to the database:

  • none

@NarrowsProjects NarrowsProjects self-assigned this Feb 20, 2026
@NarrowsProjects NarrowsProjects added frontend javascript Pull requests that update Javascript code labels Feb 20, 2026
@NarrowsProjects NarrowsProjects marked this pull request as draft February 20, 2026 11:53
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 20, 2026

Codecov Report

❌ Patch coverage is 27.11864% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.75%. Comparing base (47203c7) to head (a40f51f).

Files with missing lines Patch % Lines
...ib/public/views/Logs/Overview/LogsOverviewModel.js 0.00% 11 Missing ⚠️
...blic/views/Logs/ActiveColumns/logsActiveColumns.js 0.00% 8 Missing ⚠️
...nts/Filters/LogsFilter/author/AuthorFilterModel.js 0.00% 6 Missing ⚠️
...blic/views/Runs/ActiveColumns/runsActiveColumns.js 0.00% 5 Missing ⚠️
...mponents/Filters/LogsFilter/author/authorFilter.js 0.00% 3 Missing ⚠️
...onments/ActiveColumns/environmentsActiveColumns.js 0.00% 3 Missing ⚠️
...ic/components/Filters/common/filters/textFilter.js 0.00% 2 Missing ⚠️
...mponents/Filters/common/filters/textInputFilter.js 0.00% 2 Missing ⚠️
...ws/LhcFills/ActiveColumns/lhcFillsActiveColumns.js 0.00% 2 Missing ⚠️
...ataPasses/ActiveColumns/dataPassesActiveColumns.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2083      +/-   ##
==========================================
+ Coverage   45.50%   45.75%   +0.25%     
==========================================
  Files        1044     1042       -2     
  Lines       17353    17214     -139     
  Branches     3149     3120      -29     
==========================================
- Hits         7897     7877      -20     
+ Misses       9456     9337     -119     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@NarrowsProjects NarrowsProjects force-pushed the improv/O2B-1534/Migrate-Log-Overview-to-use-FilteringModel-pattern branch from 07567e2 to 114285d Compare February 20, 2026 12:02
@NarrowsProjects NarrowsProjects force-pushed the improv/O2B-1534/Migrate-Log-Overview-to-use-FilteringModel-pattern branch from a936a03 to 5500b3e Compare February 20, 2026 16:24
@NarrowsProjects NarrowsProjects marked this pull request as ready for review February 21, 2026 18:22
Comment thread lib/public/views/Logs/ActiveColumns/logsActiveColumns.js Outdated
Comment thread test/api/logs.test.js
Comment thread lib/public/components/Filters/LogsFilter/author/authorFilter.js Outdated
isaachilly
isaachilly previously approved these changes Apr 15, 2026
* @param {FilteringModel} logOverviewModel.filteringModel filtering model
* @return {Component} the filter component
*/
filter: ({ filteringModel }) => rawTextFilter(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about re-using the runNumbersFilter in this case?

Copy link
Copy Markdown
Collaborator Author

@NarrowsProjects NarrowsProjects Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see much use in making or using individual components for each filter that uses rawTextFilter that are virtually Identical.

What I do think is a good idea now that you mention it is to make a new component that would cover most of these usecases and change it for special cases such as authorFilter. Something like

export const textInputFilter =
    (filteringModel, key, placeholder) => rawTextFilter(filteringModel.get(key), { classes: ['w-100', `key-${filter}`], placeholder });

Which would then shorten these filter creations to:

filter: ({ filteringModel }) => textInputFilter(filteringModel, 'runNumbers', 'e.g. 553203, 553221, ...'),

* @return {Component} the filter component
*/
filter: ({ filteringModel }) => rawTextFilter(
filteringModel.get('environmentIds'),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar question, I see that runsActiveColumns also has the environmentIds as rawtextFilter
Could we perhaps extract as a reusable component similar to runNumbersFilter?

Copy link
Copy Markdown
Collaborator Author

@NarrowsProjects NarrowsProjects Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above

* @param {FilteringModel} logOverviewModel.filteringModel filtering model
* @return {Component} the filter component
*/
filter: ({ filteringModel }) => rawTextFilter(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Copy Markdown
Collaborator Author

@NarrowsProjects NarrowsProjects Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above

Comment thread lib/usecases/log/GetAllLogsUseCase.js
Comment thread lib/usecases/log/GetAllLogsUseCase.js
Comment thread lib/usecases/log/GetAllLogsUseCase.js
Comment thread lib/domain/dtos/GetAllLogsDto.js
@NarrowsProjects NarrowsProjects force-pushed the improv/O2B-1534/Migrate-Log-Overview-to-use-FilteringModel-pattern branch from a86e6a6 to a40f51f Compare April 16, 2026 17:24
@NarrowsProjects NarrowsProjects requested a review from graduta April 16, 2026 17:36
import { fillNumberFilter } from '../../../components/Filters/LhcFillsFilter/fillNumberFilter.js';
import { durationFilter } from '../../../components/Filters/LhcFillsFilter/durationFilter.js';
import { beamTypeFilter } from '../../../components/Filters/LhcFillsFilter/beamTypeFilter.js';
import { schemeNameFilter } from '../../../components/Filters/LhcFillsFilter/schemeNameFilter.js';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this file not be removed the same as runNumbersFilter? I do not see it being used anywhere else

import { formatBeamType } from '../../../utilities/formatting/formatBeamType.js';
import { frontLink } from '../../../components/common/navigation/frontLink.js';
import { toggleStableBeamOnlyFilter } from '../../../components/Filters/LhcFillsFilter/stableBeamFilter.js';
import { fillNumberFilter } from '../../../components/Filters/LhcFillsFilter/fillNumberFilter.js';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this file not be removed?

import { iconCommentSquare, iconPaperclip } from '/js/src/icons.js';

import { authorFilter } from '../../../components/Filters/LogsFilter/author/authorFilter.js';
import createdFilter from '../../../components/Filters/LogsFilter/created.js';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be removed?


import { authorFilter } from '../../../components/Filters/LogsFilter/author/authorFilter.js';
import createdFilter from '../../../components/Filters/LogsFilter/created.js';
import runsFilter from '../../../components/Filters/LogsFilter/runs.js';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be removed?

import { formatRunsList } from '../../Runs/format/formatRunsList.js';
import { profiles } from '../../../components/common/table/profiles.js';
import { textFilter } from '../../../components/Filters/common/filters/textFilter.js';
import { environmentFilter } from '../../../components/Filters/LogsFilter/environments.js';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^

import { textFilter } from '../../../components/Filters/common/filters/textFilter.js';
import { environmentFilter } from '../../../components/Filters/LogsFilter/environments.js';
import { formatLhcFillsList } from '../../LhcFills/format/formatLhcFillsList.js';
import { lhcFillsFilter } from '../../../components/Filters/LogsFilter/lhcFill.js';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend javascript Pull requests that update Javascript code

Development

Successfully merging this pull request may close these issues.

3 participants